Skip to content

fix: capture async stack only for rejections with native error object…#5

Open
MitchLewis930 wants to merge 1 commit intopr_025_beforefrom
pr_025_after
Open

fix: capture async stack only for rejections with native error object…#5
MitchLewis930 wants to merge 1 commit intopr_025_beforefrom
pr_025_after

Conversation

@MitchLewis930
Copy link

@MitchLewis930 MitchLewis930 commented Jan 29, 2026 •
edited by coderabbitai bot
Loading

PR_025

Summary by CodeRabbit

  • Bug Fixes

    • Improved HTTP error stack handling so existing stack information is preserved and appended only when appropriate, yielding more reliable diagnostics.

  • Tests

    • Updated unit tests for HTTP error behavior to use async/await patterns and promise-based assertions.

@coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2026 •
edited
Loading

📝 Walkthrough

Walkthrough

Axios request error handling was refined to only build and append a derived stack for actual Error instances, preserving existing stacks when present. A unit test was modernized from callback-style to async/await with promise-based assertions.

Changes

Cohort / File(s) Summary
Error Handling Logic
lib/core/Axios.js		 Adjust catch behavior to operate only when err is an Error instance: construct a derived stack via Error.captureStackTrace (or new Error()), strip header line, assign or append the derived tail only when appropriate; removes unconditional mutation of plain objects.
Test Modernization
test/unit/adapters/http.js		 Converted test from callback (done) style to async/await; use startHTTPServer(...) with await, and replace callback-based assertions with assert.rejects promise pattern.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nudged the stacks to tidy the trail,
No faux objects left to wag a false tail,
Tests now await with a patient delight,
Errors stand clear in the soft morning light,
Hoppity hops — everything’s right! 🥕

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing stack capture behavior to only apply to native error objects, which aligns with the core modification in Axios.js.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into pr_025_before

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pr_025_after

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 104aa3f and 1a08f90.

📒 Files selected for processing (2)
  • lib/core/Axios.js
  • test/unit/adapters/http.js
🧰 Additional context used
🧬 Code graph analysis (1)
test/unit/adapters/http.js (2)
test/unit/regression/SNYK-JS-AXIOS-1038255.js (1)
  • server (16-16)
lib/adapters/http.js (1)
  • req (167-167)
🔇 Additional comments (2)
test/unit/adapters/http.js (1)

452-474: LGTM! Clean modernization to async/await.

The refactoring from callback-style (done) to async/await with return assert.rejects() is cleaner and follows modern testing patterns. The test logic correctly validates that HTTP errors are wrapped as AxiosError with preserved async stack traces.

lib/core/Axios.js (1)

42-56: LGTM! Correctly guards stack manipulation for Error instances only.

The instanceof Error check (line 42) is the key fix that prevents stack manipulation on non-Error rejections (strings, null, plain objects, etc.), which could cause unexpected behavior. The stack capture and conditional append logic is sound:

  • Uses Error.captureStackTrace when available (V8) with appropriate fallback
  • Prevents duplicate stack appending via the endsWith check
  • Defensively converts to String() before comparison

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment @coderabbitai help to get the list of available commands and usage tips.

@MitchLewis930
Copy link
Author

MitchLewis930 commented Feb 13, 2026

@coderabbitai full review

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

✅ Actions performed

Full review triggered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MitchLewis930 @DigitalBrainJS